Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supporting multiple XFF entries #8

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

RobinNil
Copy link

@RobinNil RobinNil commented Aug 1, 2018

This is to add support/fix bugs for having multiple X-Forwarded-For entries.
The commit message has details of the implementations.

There is a bug on the test case where "X-Forwarded-For" headers were not added (but were replaced instead):
master...RobinNil:master#diff-ffc776942d64deeeecf0f5ff91c039bfL55

Robin Robin and others added 4 commits July 31, 2018 21:41
r.Header.Set simply overwrites the values.
The expected public IP should be the first public IP encountered.
When we use Header.Get only the first entry is fetched.

Per https://golang.org/src/net/http/request.go,
when there are multiple entries of the same header,
accessing the map directly allows us to get those
multiple entries in an array.

// Header contains the request header fields either received
// by the server or to be sent by the client.
//
// If a server received a request with header lines,
//
//	Host: example.com
//	accept-encoding: gzip, deflate
//	Accept-Language: en-us
//	fOO: Bar
//	foo: two
//
// then
//
//	Header = map[string][]string{
//		"Accept-Encoding": {"gzip, deflate"},
//		"Accept-Language": {"en-us"},
//		"Foo": {"Bar", "two"},
XFF can contain IP1,IP2,IP3
XFF can contain IP1, IP2 (note the space)
feat: Support multiple XFF headers
@RobinNil
Copy link
Author

RobinNil commented Aug 1, 2018

@tomasen could you help review? thanks.

@flyinprogrammer
Copy link

This PR addresses all the concerns in #5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants